Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/tinydtls: add IPv4 support #17765

Merged
merged 3 commits into from
Mar 26, 2022

Conversation

HendrikVE
Copy link
Contributor

@HendrikVE HendrikVE commented Mar 7, 2022

Contribution description

Add IPv4 support to pkg/tinydtls.

This PR contains two patches which I will try to get merged in the upstream repository once this PR is merged.

Testing procedure

Can be tested with #17763

Issues/PRs references

Dependencies:

@github-actions github-actions bot added the Area: pkg Area: External package ports label Mar 7, 2022
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 9, 2022
&& uip_ipaddr_cmp(&((A)->addr),&((B)->addr)) \
&& (A)->ifindex == (B)->ifindex)
-#elif defined(WITH_RIOT_GNRC)
+#elif defined(WITH_RIOT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
+#elif defined(WITH_RIOT)
#elif defined(RIOT_VERSION)

should work as well and doesn't require a custom define (this is used by other pkgs too)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah wait so when RIOT is used with the posix_sockets module this path should not be taken - then how about WITH_RIOT_SOCK

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed WITH_RIOT_GNRC to WITH_RIOT_SOCK as suggested.

@OlegHahm OlegHahm added this to the Release 2022.04 milestone Mar 9, 2022
@benpicco
Copy link
Contributor

benpicco commented Mar 9, 2022

CI is unhappy

@HendrikVE HendrikVE force-pushed the pr/pkg_tinydtls_ipv4_support branch from f979699 to 7def280 Compare March 10, 2022 16:05
@github-actions github-actions bot added the Area: examples Area: Example Applications label Mar 10, 2022
@HendrikVE
Copy link
Contributor Author

I fixed the compilation for examples/dtls-echo. It does not make use of IPv4 yet. The failed test is unrelated to these changes, otherwise CI seems happy now.

Using tinydtls on RIOT OS is not limited to GNRC as network stack.
It is also working with e.g. lwIP, see: RIOT-OS#17552
Therefore the name WITH_RIOT_GNRC is misleading.
@HendrikVE HendrikVE force-pushed the pr/pkg_tinydtls_ipv4_support branch from 7def280 to d424aae Compare March 25, 2022 16:21
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
Btw: tinydtls has RIOT support, so you also want to submit your patch upsream so we can drop it in the future.

@HendrikVE
Copy link
Contributor Author

I created PRs accordingly:

@benpicco benpicco merged commit 2e51328 into RIOT-OS:master Mar 26, 2022
@HendrikVE
Copy link
Contributor Author

Thank you very much for your review @benpicco !

@miri64
Copy link
Member

miri64 commented Jul 26, 2022

This PR broke examples/dtls-sock. See #17765.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants